Skip to content

analytics: Charge driver verdicts agree with the row's printed values and points - #2501

Closed
andremiliano wants to merge 1 commit into
ryanbr:mainfrom
andremiliano:analytics/charge-verdicts
Closed

andremiliano wants to merge 1 commit into
ryanbr:mainfrom
andremiliano:analytics/charge-verdicts

Conversation

@andremiliano

Copy link
Copy Markdown

What this PR does

Makes each Charge "What shaped it" verdict agree with what its own row prints (#2466).

A driver row prints the value and its baseline at display precision, plus the term's rounded points. The verdict, however, compared the raw values. So a row could read 51 bpm · 51 bpm baseline · 0 pts — above baseline, limiting recovery, and skin temperature could say near baseline beside −1 pt.

Now:

  • Effect (supporting / limiting) comes from the row's rounded points.
  • Direction (above / below) comes from the values at the precision the row prints them.
  • If the printed values tie but the term still moved the score: slightly above/below baseline, …
  • If a printed difference is worth 0 points: above/below baseline, too small to change Charge
  • Skin temperature is near baseline exactly when it cost 0 points.

Scores and points are unchanged. Only the verdict wording is selected differently.

Type of change

  • Bug fix

How it was tested

  • Swift: swift test in Packages/StrandAnalytics passes (2067 tests). This includes a new testVerdictsAgreeWithThePrintedValuesAndPoints covering each case above. The Not working on macOS with Intel processor #51 half-tie pin moves to slightly below baseline, limiting recovery, since it prints 30 ms against a 30 ms baseline at −1 pt.
  • Kotlin parity, by oracle: I ran the Swift chargeDrivers over a 288-case grid and pasted its output verbatim into the new ChargeDriverVerdictOracleTest. The grid covers HRV, resting HR and respiration either side of baseline and on the display-rounding edges, skin drift inside and outside the old ±0.3 °C band, and two sleep qualities. The Kotlin twin reproduces every line. That test also asserts that every ChargeDriverVerdict maps to a Swift catalog string.
  • Android: ./gradlew testFullDebugUnitTest shows 6466 tests. Locally, 2 fail on this Mac: RecoveryDriversTest.driverPointRoundingUsesNearestWithHalfTiesAwayFromZero and issue51NegativeHalfTie…. Both fail identically on unmodified main on this machine (expected:<-0.5> but was:<-0.4999999999999929>, before any verdict assertion), so I'm relying on CI's Linux runner for those two. compileFullDebugKotlin succeeds.
  • Other checks:
    • Tools/i18n_audit.py --ci origin/main passes.
    • test_home_i18n passes, with its verdict-key pin moved from 12 to 18.
    • doc_comment_lint passes.
    • parity_ratchet.py --base origin/main --offline reports only the "base authority cannot be reproduced" error, which pristine main reports identically, plus a "debt decreased" warning.
  • Rounding match: the Kotlin side rounds the printed values half away from zero, to match Swift's .rounded(). It's a local value inside chargeDrivers, like the existing points helper, so it adds no unpaired declaration.
  • No BLE or hardware path touched.

Checklist

  • Swift package tests pass for any package I touched
  • Android unit tests pass if I touched android/ (except the two pre-existing local float failures noted above)
  • No new build warnings introduced
  • UI changes use only design tokens (no UI changes beyond six new strings)
  • No hardcoded hex frame bytes
  • Follows the conventions in docs/CONTRIBUTING.md
  • No generated output or secrets committed

Translations for the six new strings are machine-assisted and follow each locale's existing verdict wording. I haven't had native speakers review them.

Related issues

Refs #2466

🤖 Generated with Claude Code

… and points

A driver row prints the value and its baseline at display precision and the term's rounded points,
but the verdict compared the raw values. So a row could read "51 bpm, 51 bpm baseline, 0 pts" and say
"above baseline, limiting recovery", and skin temperature could say "near baseline" beside -1 pt.

The effect (supporting / limiting) now comes from the points, and the direction from the printed
values. When the printed values tie but the term still moved the score, the verdict says "slightly"
above or below; when a printed difference is worth 0 points it says the difference is too small to
change Charge; skin temperature is near baseline exactly when it cost nothing.

Both platforms: ChargeDrivers.swift and its Kotlin twin RecoveryDrivers.kt. Six new verdicts, as
catalog keys (nine locales) and Android string resources (eight locales), with the verdict-catalog
contract pin moving from 12 to 18. Parity is pinned by a Swift oracle: the Swift chargeDrivers output
over a 288-case grid (values either side of baseline and on the display-rounding edges, skin drift
inside and outside the old band) is pasted verbatim into ChargeDriverVerdictOracleTest, which the
Kotlin twin reproduces line for line. The existing ryanbr#51 half-tie expectation moves to "slightly below
baseline, limiting recovery" on both sides: it prints 30 ms against a 30 ms baseline at -1 pt.

Refs ryanbr#2466

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ryanbr

ryanbr commented Sep 27, 2026

Copy link
Copy Markdown
Owner

Superseded on the main fix, but not on all of it, so please read the second half before writing this off.

Thanks @andremiliano. #2466 was one of the most useful reports in a while: it named three separate contradictions, located each one by file and line, worked out that the verdict was comparing raw values while the row printed rounded ones, and predicted that the verdict-catalog pin would go from 12 to 18. It did. Back-solving your 14/100 figure through 100 × ln(TRIMP+1) / ln(7201) also landed where you said, at roughly 2.5 zone-weighted minutes above 50 % HRR, which is how a chip meant for hard days fired on almost every day with movement.

That work landed from #2468 via #2500, as 6151113 and a0bf1b6, before this arrived: effect from the rounded points, direction from the displayed precision, and the pin moved to 18. Your suggested 70-adds and 60-does-not values are what shipped. Two independent people solving the same report is a good problem to have, and the only reason yours is the one being retired is order of arrival.

One part of this is NOT superseded and is worth having on its own: ChargeDriverVerdictOracleTest.kt. That is the oracle AGENTS.md actually asks for and the merged work does not include one for the verdict mapping. Swift stdout pasted verbatim over a grid that spans the display-rounding edges, rows as label/points/verdict, so the Kotlin twin has to reproduce every line rather than be read alongside the Swift by eye. The merged change only pinned displayRounded that way, which is a much narrower contract than the whole mapping. A focused PR with just that file would go in on its merit; say the word if you would rather I lifted it with attribution instead.

Also still open ground, from your own "related, smaller" note: the classic Today breakdown states skin temperature twice, once as a driver row and once as the relative row via skinTempRel, in different words. Neither this nor the merged change touches it, and it wants a decision about which of the two should go rather than a patch.

@ryanbr ryanbr closed this Sep 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants